test: withr-free local seed and RNG-state leak fixes by schochastics · Pull Request #2714 · igraph/rigraph

schochastics · 2026-06-25T14:45:01Z

What & why

The test suite has an opt-in check (gated behind IGRAPH_CHECK_RNG_STATE=true in
tests/testthat/setup.R) that fails any test which leaves the global
.Random.seed changed. This PR makes the whole suite pass that check and moves
the suite's RNG-seed handling off withr onto in-house helpers.

Running the suite with IGRAPH_CHECK_RNG_STATE=true: 0 seed leaks, 0
failures, 0 warnings (9739 passing).

New helpers (`tests/testthat/helper.R`)

igraph_local_seed() — drop-in for withr::local_seed(). Sets the seed for
the current scope and restores the previous global RNG state when that scope
exits.

Self-contained: snapshots and restores RNGkind + .Random.seed itself
(uses withr::defer only as the deferral primitive), so it doesn't inherit
withr::local_seed's past seed-handling bugs.
Restores RNGkind first, then .Random.seed last — changing the kind
discards the seed, so the seed write must come last for an exact round-trip.
Optional rng_version (e.g. "3.5.0"), folding in what used to be a separate
local_rng_version() call. Pairing two independent deferred restores (kind +
seed) ran them in the wrong order and leaked; a single ordered restore fixes it.

igraph_with_seed() — block form mirroring withr::with_seed(): set the
seed, evaluate the block, restore. Needed where a single test has several
independently seeded blocks with code between them (e.g. test-print-classic.R),
which a frame-scoped igraph_local_seed() can't express.

setup.R

Set a baseline seed (set.seed(42)) so .Random.seed always exists before
tests run. Otherwise the first RNG-touching test creates it from nothing
(many igraph C functions initialise the RNG without drawing from it, e.g.
voronoi_cells()), which the check flagged — and restoring that absent state
just shoved the "first creation" onto the next test (whack-a-mole).
The seed-leak override now forwards the test block via
substitute() + eval(as.call(...)) instead of
rlang::inject(testthat::test_that(name, !!code)). inject() eagerly
processed any injection operator in the test body (!!, !!!, {{),
e.g. set_vertex_attrs(g, !!!attr_list), evaluating it in the wrong frame and
erroring with object 'attr_list' not found.
The check stays gated behind IGRAPH_CHECK_RNG_STATE=true (opt-in for CI).

Test files

Moved helper functions that were defined inside test files into a shared
helper-test-functions.R (the override re-scopes test code, so file-local
helpers were no longer reachable from the test body).
Added igraph_local_seed() to tests that genuinely advance the RNG (unseeded
sample_* fixtures and ARPACK eigensolvers).
Replaced all 875 withr::local_seed() call sites with
igraph_local_seed() (including the 732 in test-aaa-auto.R).
Replaced the 9 withr::with_seed() call sites with igraph_with_seed()
(test-print-classic.R, test-layout.R, test-iterators.R). These never
leaked — consistency cleanup; snapshots unchanged.
Replaced every local_rng_version("3.5.0") + seed pairing with
igraph_local_seed(seed, rng_version = "3.5.0"), then removed the now-unused
local_rng_version() helper.
restore_rng_state() suppresses warnings around the RNGkind restore so
putting back the older "Rounding" sampler (RNGversion("3.5.0")) doesn't
emit R's "non-uniform 'Rounding' sampler used" note.

tools/AGENTS.md

Updated the test-writing guidance to use igraph_local_seed, so newly added
test-aaa-auto.R tests follow the convention.

Notes

igraph_local_seed()/igraph_with_seed() live in helper.R (not setup.R)
so helper functions like make_scan_graphs() can see them.
The suite no longer uses withr for any RNG-seed handling.

🤖 Generated with Claude Code

The test_that override in setup.R fails any test that changes the global .Random.seed (gated behind IGRAPH_CHECK_RNG_STATE). Several things had to change for the suite to pass under it: * Move helper functions defined inside test files into a shared helper-test-functions.R. The override re-scopes test code, so file-local helpers were no longer reachable from the test body. * Add igraph_local_seed() (helper.R): a withr::local_seed-free local seed that snapshots and restores RNGkind + .Random.seed itself, restoring the kind first and the seed last so the state round-trips exactly. It takes an optional rng_version, replacing the leaky local_rng_version() + withr::local_seed() pairing (whose two independent deferred restores ran in the wrong order). * setup.R: set a baseline seed so .Random.seed always exists -- otherwise the first RNG-touching test creates it from nothing (many igraph C functions init the RNG without drawing, e.g. voronoi_cells()), which the check flagged. Also forward the test block via substitute()/eval() instead of rlang::inject(testthat::test_that(name, !!code)); inject() mangled test bodies containing !!/!!!/{{ (e.g. set_vertex_attrs(g, !!!attr_list)). * Add igraph_local_seed() to the tests that genuinely advance the RNG (unseeded sample_* fixtures and ARPACK eigensolvers), and convert the local_rng_version() + withr::local_seed() pairs to igraph_local_seed(rng_version = "3.5.0"). Full suite passes the check: 0 seed leaks. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Migrate all 875 withr::local_seed() call sites in the test suite to igraph_local_seed(), completing the move off withr for RNG-seed handling. All calls passed only a seed, so this is a direct swap; igraph_local_seed() restores RNGkind + .Random.seed itself (a superset of withr::local_seed's behaviour). Also: * restore_rng_state() now suppresses warnings around the RNGkind restore, so putting back an older sampler (the "Rounding" sample.kind from RNGversion("3.5.0")) no longer emits R's "non-uniform 'Rounding' sampler used" note. This surfaced once the inner withr::local_seed calls nested under igraph_local_seed(rng_version = "3.5.0") were migrated; withr had suppressed it. * tools/AGENTS.md: update the test-writing guidance to use igraph_local_seed so newly added test-aaa-auto.R tests follow the convention. Full suite passes the seed-leak check with no warnings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

…e checker

Add igraph_with_seed(), a block-form companion to igraph_local_seed() that mirrors withr::with_seed(): set the seed, evaluate the block, restore the prior global RNG state. It is needed where a single test has several independently seeded blocks with code between them (e.g. test-print-classic.R), which a frame-scoped igraph_local_seed() can't express. Migrate the 9 withr::with_seed() call sites (test-print-classic.R, test-layout.R, test-iterators.R) to igraph_with_seed(), completing the removal of withr from the suite's RNG-seed handling. These never leaked (with_seed is self-contained); this is consistency cleanup. Snapshots unchanged; suite passes the seed-leak check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Dead code: every local_rng_version("3.5.0") call was folded into igraph_local_seed(seed, rng_version = "3.5.0") earlier in this branch, leaving no call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

maelle

Thank you!! 🎲

maelle · 2026-07-03T12:10:41Z

+# optionally pins the RNG version via `rng_version`, e.g. "3.5.0") and restores
+# the pre-call global RNG state when `.local_envir` exits (the current test_that
+# block, or the enclosing helper function when called from one). Self-contained:
+# it does its own save/restore, so it does not depend on withr's local_seed().


link to some withr issue to explain our choice?

maelle · 2026-07-03T12:14:37Z

+# does when `before` was NULL) just pushes the creation onto the next test -- a
+# whack-a-mole. Pinning a baseline here means only tests that genuinely *advance*
+# the seed need igraph_local_seed().
+set.seed(42)


and this seed never leaks in an user's interactive session?

No this doesn leak. Test run in there own R throwaway R process

Can be removed now entirely?

The baseline exists for a different class of tests: ones that are not wrapped (because they aren't "random" tests) but call an igraph C function that defensively initialises R's RNG via GetRNGstate/PutRNGstate without ever drawing. Those functions:

create .Random.seed from nothing when none exists → the leak check flags it, but
leave an existing .Random.seed unchanged.
set.seed(42) guarantees .Random.seed always exists, so those init-only calls read-and-write-back the same value and don't get flagged — which means such tests don't need a pointless igraph_local_seed() wrapper.

… restore_rng_state after igraph_with_seed, consolidate rlang::inject caveat into inline comment, and gate seed-leak check documentation

krlmlr

Thanks, please merge when green.

krlmlr · 2026-07-09T08:50:14Z

+# does when `before` was NULL) just pushes the creation onto the next test -- a
+# whack-a-mole. Pinning a baseline here means only tests that genuinely *advance*
+# the seed need igraph_local_seed().
+set.seed(42)


Can be removed now entirely?

Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>

schochastics and others added 5 commits June 25, 2026 15:12

Re-enable IGRAPH_CHECK_RNG_STATE env var guard for test_that RNG stat…

7d9cec9

…e checker

test: remove unused local_rng_version() helper

f1ca41f

Dead code: every local_rng_version("3.5.0") call was folded into igraph_local_seed(seed, rng_version = "3.5.0") earlier in this branch, leaving no call sites. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

schochastics added the hackathon-topic 📆 label Jul 3, 2026

schochastics requested a review from maelle July 3, 2026 09:31

maelle reviewed Jul 3, 2026

View reviewed changes

Refactor test helper comments and reorder functions for clarity; move…

b2d66a7

… restore_rng_state after igraph_with_seed, consolidate rlang::inject caveat into inline comment, and gate seed-leak check documentation

krlmlr reviewed Jul 9, 2026

View reviewed changes

schochastics and others added 2 commits July 9, 2026 11:34

Update tests/testthat/helper.R

b3ff7b6

Co-authored-by: Kirill Müller <krlmlr@users.noreply.github.com>

Merge branch 'main' into rng-state-seed-leak-check

9d5fab9

schochastics enabled auto-merge (squash) July 9, 2026 09:42

auto-merge was automatically disabled July 9, 2026 09:53
Repository rule violations found

schochastics merged commit 84c76a4 into main Jul 9, 2026
6 checks passed

schochastics deleted the rng-state-seed-leak-check branch July 9, 2026 10:02

Uh oh!

Uh oh!

Conversation

schochastics commented Jun 25, 2026 • edited by maelle Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

New helpers (tests/testthat/helper.R)

setup.R

Test files

tools/AGENTS.md

Notes

Uh oh!

maelle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maelle Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maelle Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

schochastics Jul 3, 2026

Choose a reason for hiding this comment

Uh oh!

krlmlr Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

schochastics Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

krlmlr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krlmlr Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

schochastics commented Jun 25, 2026 •

edited by maelle

Loading

New helpers (`tests/testthat/helper.R`)